Conversation
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughResponsive UI adjustments across sender and receiver components, a mobile-only 30‑segment SVG circular progress ring added to the transfer UI, layout heights switched to dynamic viewport units, filename truncation and small responsive visibility tweaks, and a JSON schema file reformatted for whitespace only. Changes
Possibly related PRs
🚥 Pre-merge checks | ❌ 1❌ Failed checks (1 warning)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
web-app/src/components/common/TransferSuccessScreen.tsx (1)
158-163:⚠️ Potential issue | 🟠 MajorPlease keep the
Openaction available on small screens.Line 163 hides the button for every viewport below
sm, which removes the only folder-opening action even whenonOpenFolderis available. That’s a functionality regression in narrow/mobile layouts; a stacked layout would preserve the action without crowding the UI.💡 Responsive fix
- <div className="flex gap-3 w-full max-w-sm"> + <div className="flex flex-col sm:flex-row gap-3 w-full max-w-sm"> <Button type="button" variant="secondary" onClick={onOpenFolder} - className="flex-1 hidden sm:flex" + className="w-full sm:flex-1" >web-app/src/components/sender/Dropzone.tsx (1)
70-76:⚠️ Potential issue | 🟡 MinorMinor: Trailing space in className.
There's a trailing space in the className string that can be cleaned up.
🧹 Suggested fix
- <span className="-mr-2 hidden sm:block "> + <span className="-mr-2 hidden sm:block">
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8589849a-12ed-4f53-941b-c555c87d8be6
📒 Files selected for processing (12)
src-tauri/plugins/tauri-plugin-native-utils/permissions/schemas/schema.jsonweb-app/src/components/common/PulseAnimation.tsxweb-app/src/components/common/TransferProgressBar.tsxweb-app/src/components/common/TransferSuccessScreen.tsxweb-app/src/components/layouts/RootLayout.tsxweb-app/src/components/receiver/Receiver.tsxweb-app/src/components/sender/BrowseButtons.tsxweb-app/src/components/sender/Dropzone.tsxweb-app/src/components/sender/Sender.tsxweb-app/src/components/sender/SharingActiveCard.tsxweb-app/src/components/ui/frame.tsxweb-app/src/routes/index.tsx
| <p className=" hidden sm:block text-lg font-medium mb-2 text-accent-foreground"> | ||
| {getStatusText()} | ||
| </p> |
There was a problem hiding this comment.
Minor: Leading space in className.
There's a leading space before "hidden" in the className string.
🧹 Suggested fix
- <p className=" hidden sm:block text-lg font-medium mb-2 text-accent-foreground">
+ <p className="hidden sm:block text-lg font-medium mb-2 text-accent-foreground">|
|
||
| return ( | ||
| <div className="p-6 space-y-6 relative h-112 overflow-y-auto flex flex-col"> | ||
| <div className="p-0 sm:p-6 space-y-6 relative h-[62dvh] sm:h-112 overflow-y-auto flex flex-col"> |
There was a problem hiding this comment.
Avoid introducing a second scroll container here.
Line 95 combines a fixed h-[62dvh] with overflow-y-auto, but the surrounding page in web-app/src/components/common/SingleLayoutPage.tsx:8-12 already owns vertical scrolling. With body locked to overflow: hidden, this creates nested scrolling on mobile and makes the sender panel easy to “trap” while swiping. Prefer letting the outer layout scroll and using min-h/flex sizing here instead of a fixed inner scroller.
Suggested direction
- <div className="p-0 sm:p-6 space-y-6 relative h-[62dvh] sm:h-112 overflow-y-auto flex flex-col">
+ <div className="p-0 sm:p-6 space-y-6 relative min-h-[62dvh] sm:h-112 flex flex-col">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div className="p-0 sm:p-6 space-y-6 relative h-[62dvh] sm:h-112 overflow-y-auto flex flex-col"> | |
| <div className="p-0 sm:p-6 space-y-6 relative min-h-[62dvh] sm:h-112 flex flex-col"> |
| (viewState === 'SHARING' && isBroadcastMode)) && ( | ||
| <> | ||
| <div className="text-center"> | ||
| <div className="text-center mt-18 sm:mt-0"> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Replace the magic top offset with layout-driven alignment.
Line 140 hard-codes vertical placement with mt-18, which is fragile on short or landscape screens and with longer localized copy. A flex-based centering strategy on the active-state wrapper will scale better than spending a fixed chunk of the mobile height budget on margin.
| <p className="text-xs mb-4 max-w-40 sm:max-w-120 truncate"> | ||
| <strong className="mr-1">{t('common:sender.fileLabel')}</strong>{' '} | ||
| {selectedPath?.split('/').pop()} | ||
| </p> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Search for max-w-120 usage and check if there's custom Tailwind config extending max-width
# Check if max-w-120 is used elsewhere in the codebase
echo "=== Searching for max-w-120 usage ==="
rg -n 'max-w-120' --type-add 'web:*.{tsx,jsx,ts,js,css}' --type web
# Check Tailwind config for custom max-width extensions
echo "=== Checking Tailwind configuration ==="
fd -e js -e ts -e mjs 'tailwind.config' --exec cat {}Repository: tonyantony300/alt-sendme
Length of output: 265
🏁 Script executed:
#!/bin/bash
# Search for tailwind config in web-app directory specifically
echo "=== Searching for Tailwind config in web-app ==="
fd -H 'tailwind' web-app --type f | head -20
# Check package.json for Tailwind version
echo "=== Checking Tailwind version in web-app ==="
if [ -f "web-app/package.json" ]; then
jq '.dependencies | to_entries[] | select(.key | contains("tailwind"))' web-app/package.json
fi
# Check root package.json as well
echo "=== Checking Tailwind version in root ==="
if [ -f "package.json" ]; then
jq '.dependencies, .devDependencies | to_entries[] | select(.key | contains("tailwind"))' package.json 2>/dev/null || true
fi
# Look for any CSS config or build tools
echo "=== Searching for PostCSS/build configs ==="
fd -H '\.(postcss|css\.js|tailwind)' web-app --type fRepository: tonyantony300/alt-sendme
Length of output: 405
🏁 Script executed:
#!/bin/bash
# Check for other max-w usage patterns
echo "=== Searching for max-w usage in codebase ==="
rg -n 'max-w-' --type-add 'web:*.{tsx,jsx,ts,js}' --type web | head -30
# Check for arbitrary Tailwind values using bracket syntax
echo "=== Searching for arbitrary Tailwind values (bracket syntax) ==="
rg -n '\[.*\]' web-app/src/components --type tsx | grep -E '(max-w|max-h|w-|h-)' | head -20
# Check if there's a config extending theme
echo "=== Checking for theme extensions ==="
rg -n 'theme|extend' web-app --type ts --type js | head -20
# Check for any Tailwind warnings in tsconfig or other config
echo "=== Checking TypeScript or build config ==="
fd -H 'tsconfig\|next.config\|vite.config\|webpack' web-app --type f -x cat {}Repository: tonyantony300/alt-sendme
Length of output: 17026
max-w-120 is not a valid Tailwind utility and needs correction.
Tailwind v4.1.18 doesn't include numeric utilities beyond the standard scale (which tops out at max-w-7xl). The value max-w-120 will be silently ignored and won't apply any width constraint. Use an arbitrary value instead: max-w-[30rem] (equivalent to 120 × 0.25rem = 480px) or max-w-[480px]. The responsive pattern itself—constraining width on mobile while expanding on larger screens—makes sense for the truncated file path.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web-app/src/components/receiver/ReceivingActiveCard.tsx (1)
51-60: 🧹 Nitpick | 🔵 TrivialConsider removing the redundant dark mode utility class.
The className includes both
dark:not-disabled:before:shadow-noneanddark:not-disabled:not-active:not-data-pressed:before:shadow-none. Since the former applies whenevernot-disabledis true in dark mode, it already covers all cases where the latter would apply (the additional:not-active:not-data-pressedconditions are a subset). You could safely remove the more specific variant to reduce class complexity.🧹 Suggested simplification
<Button variant={'destructive-outline'} size="icon-lg" type="button" onClick={onStopReceiving} - className="absolute top-0 right-2 sm:right-6 rounded-full font-medium transition-colors not-disabled:not-active:not-data-pressed:before:shadow-none dark:not-disabled:before:shadow-none dark:not-disabled:not-active:not-data-pressed:before:shadow-none" + className="absolute top-0 right-2 sm:right-6 rounded-full font-medium transition-colors not-disabled:not-active:not-data-pressed:before:shadow-none dark:not-disabled:before:shadow-none" aria-label="Stop receiving" >
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4cb6620a-e657-4cf5-a5fd-4834972f0709
📒 Files selected for processing (3)
web-app/src/components/receiver/Receiver.tsxweb-app/src/components/receiver/ReceivingActiveCard.tsxweb-app/src/routes/index.tsx
| variant="ghost" | ||
| onClick={() => setShowInstructionsDialog(true)} | ||
| className="absolute top-6 right-6" | ||
| className="absolute top-0 right-0 sm:top-6 sm:right-6" |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider slight offset for the info button on mobile.
With the container padding reduced to p-2 on mobile and the button now at top-0 right-0, the info button sits right at the container's edge. This is functional, but you might find it feels a bit tight visually.
If it looks cramped during testing, a small offset like top-1 right-1 could provide a touch more breathing room while still being compact. That said, if it looks good on your test devices, feel free to keep it as-is!
2 participants
Description
Checklist
npm run lintbefore raising this PRnpm run formatbefore raising this PR